Update Python typings #139

piotr-roslaniec · 2023-07-10T10:12:15Z

Update README if needed post Update README.md #152

Type of PR:

Bugfix

Required reviews:

2

What this does:

Fixes some Python typings not matching their runtime definition
Adds mypy.stubtest check to CI
Runs umbral-pre-python/example/example.py on CI
Renames ferveo_py to ferveo module
Removes dependency on maturing for releasing
Removes maturin.yml CI workflow
Minor change to FerveoVariant Python type

Why it's needed:

Explain how this PR fits in the greater context of the NuCypher Network.
E.g., if this PR address a nucypher/productdev issue, let reviewers know!

Notes for reviewers:

Downstream changes: Update ferveo Python typings nucypher-core#71
Removing maturin on the back of these changes
- maturin built modules are not recognized by mypy.stubtest for some reason
- Switching to setup compatible with how we release Python packages in rust-umbral and nucypher-core
- We don't plan maintain ferveo Python package anyway; We'd rather use bindings exposed from nucypher-core
MINOR version bump

codecov-commenter · 2023-07-10T10:41:49Z

Codecov Report

Merging #139 (0e7c561) into main (ffb9b21) will decrease coverage by 0.52%.
The diff coverage is 7.40%.

@@            Coverage Diff             @@
##             main     #139      +/-   ##
==========================================
- Coverage   78.48%   77.96%   -0.52%     
==========================================
  Files          24       23       -1     
  Lines        4954     4979      +25     
==========================================
- Hits         3888     3882       -6     
- Misses       1066     1097      +31

Impacted Files	Coverage Δ
ferveo-python/src/lib.rs	`0.00% <0.00%> (-20.00%)`	⬇️
ferveo/src/api.rs	`89.96% <0.00%> (-1.08%)`	⬇️
ferveo/src/bindings_wasm.rs	`0.00% <0.00%> (ø)`
ferveo/src/bindings_python.rs	`51.98% <11.42%> (-1.47%)`	⬇️

... and 5 files with indirect coverage changes

ferveo-python/Cargo.toml

KPrasch · 2023-07-11T10:04:33Z

ferveo-python/ferveo/__init__.pyi

 class DecryptionSharePrecomputed:
    @staticmethod
-    def from_bytes(data: bytes) -> DecryptionSharePrecomputed:
+    def from_bytes(bytes: bytes) -> DecryptionSharePrecomputed:


I made an identical comment in nucypher/nucypher-core#72, but I'd prefer to avoid name shadowing bytes() throughout this changeset.

I will backport these changes to nucypher-core (similar to any changes that may happen with rust_umbral)

KPrasch · 2023-07-11T10:04:49Z

ferveo-python/ferveo/__init__.pyi

 ) -> bytes:
    ...


 def combine_decryption_shares_precomputed(
-        decryption_shares: Sequence[DecryptionSharePrecomputed],
+        shares: Sequence[DecryptionSharePrecomputed],


KPrasch · 2023-07-11T10:05:55Z

ferveo-python/test/test_ferveo.py

+    (1, FerveoVariant.simple),
+    (4, FerveoVariant.simple),
+    (8, FerveoVariant.simple),
+    (32, FerveoVariant.simple),
+    (1, FerveoVariant.precomputed),
+    (4, FerveoVariant.precomputed),
+    (8, FerveoVariant.precomputed),
+    (32, FerveoVariant.precomputed),


Let's introduce some odd numbers to this parametrization, after it's supported (now?).

ferveo-python/ferveo/__init__.pyi

ferveo/src/bindings_python.rs

KPrasch

👨🏻‍🚀

derekpierre · 2023-07-13T12:14:14Z

ferveo/src/bindings_python.rs

-    fn precomputed() -> &'static str {
-        api::FerveoVariant::Precomputed.as_str()
+    fn precomputed() -> FerveoVariant {
+        api::FerveoVariant::Precomputed.into()


🔥 - this seems much better than using strings.

derekpierre · 2023-07-13T12:15:44Z

ferveo-python/test/test_serialization.py

-    assert FerveoVariant.simple() == "FerveoVariant::Simple"
+    assert str(FerveoVariant.precomputed) == "FerveoVariant::Precomputed"
+    assert str(FerveoVariant.simple) == "FerveoVariant::Simple"
+    assert FerveoVariant.precomputed == FerveoVariant.precomputed


derekpierre · 2023-07-13T12:17:02Z

ferveo/src/bindings_wasm.rs

-    pub fn precomputed() -> String {
-        api::FerveoVariant::Precomputed.as_str().to_string()
+    pub fn precomputed() -> FerveoVariant {
+        FerveoVariant(api::FerveoVariant::Precomputed)


derekpierre

🎸

piotr-roslaniec · 2023-07-13T13:16:20Z

These changes are confirmed to be working upstream, so I'd be happy to merge this once nucypher/nucypher#3171 is reviewed

derekpierre · 2023-07-13T13:22:09Z

ferveo-python/ferveo/__init__.pyi

 class FerveoVariant:
-    @staticmethod
-    def simple() -> str: ...
+    simple: FerveoVariant


One minor nitpick I didn't pick up on earlier (and I know it's annoying), but can these be SIMPLE/PRECOMPUTED or Simple/Precomputed. The lowercase looks a little weird in the Python code 😅

These are class attributes, so I assumed they should be lowercase. I can fix this.

Ah I see. Admittedly, it's a nitpick so if it is easy to fix/makes sense then great; if not, then you can ignore.

I didn't notice it until I saw it used in the 3171.

piotr-roslaniec · 2023-08-01T09:13:02Z

Tested in nucypher/nucypher#3171

fix: python typings don't match runtime

be90065

piotr-roslaniec changed the title ~~fix: python typings don't match runtime~~ Update Python typings Jul 10, 2023

piotr-roslaniec mentioned this pull request Jul 10, 2023

Update ferveo Python typings nucypher/nucypher-core#71

Closed

Base automatically changed from development to main July 10, 2023 10:18

chore(ci): remove maturin

4fcc7c0

piotr-roslaniec marked this pull request as ready for review July 10, 2023 10:43

piotr-roslaniec requested review from fjarri, KPrasch, theref, derekpierre and manumonti July 10, 2023 10:44

feat: replace FerveoVariant static methods with class atributed

50511ff

KPrasch reviewed Jul 11, 2023

View reviewed changes

piotr-roslaniec added 2 commits July 11, 2023 12:22

apply pr suggestions

7cbe65d

add api conversion method to FerveoVariant

fbb97be

derekpierre reviewed Jul 11, 2023

View reviewed changes

ferveo-python/ferveo/__init__.pyi Outdated Show resolved Hide resolved

derekpierre reviewed Jul 11, 2023

View reviewed changes

ferveo/src/bindings_python.rs Outdated Show resolved Hide resolved

piotr-roslaniec force-pushed the fix-typings branch from 583b7e0 to 9bcce6a Compare July 12, 2023 10:10

apply pr suggestions

6c1d4be

piotr-roslaniec force-pushed the fix-typings branch from 9bcce6a to 6c1d4be Compare July 12, 2023 10:17

KPrasch approved these changes Jul 12, 2023

View reviewed changes

piotr-roslaniec requested a review from derekpierre July 13, 2023 07:03

derekpierre reviewed Jul 13, 2023

View reviewed changes

piotr-roslaniec force-pushed the fix-typings branch from c3b7497 to 922ae5a Compare July 13, 2023 12:15

derekpierre reviewed Jul 13, 2023

View reviewed changes

derekpierre approved these changes Jul 13, 2023

View reviewed changes

add equality to FerveoVariant python bindings

cea467e

piotr-roslaniec force-pushed the fix-typings branch from 922ae5a to cea467e Compare July 13, 2023 12:21

derekpierre reviewed Jul 13, 2023

View reviewed changes

piotr-roslaniec added 2 commits July 14, 2023 08:45

add __hash__ to FerveoVariant

06321d7

rename FerveoVariant attributes

0e7c561

piotr-roslaniec merged commit dc9d81a into main Aug 1, 2023
9 checks passed

piotr-roslaniec deleted the fix-typings branch August 1, 2023 09:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Python typings #139

Update Python typings #139

piotr-roslaniec commented Jul 10, 2023 •

edited

Loading

codecov-commenter commented Jul 10, 2023 •

edited

Loading

KPrasch Jul 11, 2023

piotr-roslaniec Jul 11, 2023

KPrasch Jul 11, 2023

KPrasch Jul 11, 2023

piotr-roslaniec Jul 11, 2023

KPrasch left a comment

derekpierre Jul 13, 2023

derekpierre Jul 13, 2023

derekpierre Jul 13, 2023

derekpierre left a comment

piotr-roslaniec commented Jul 13, 2023

derekpierre Jul 13, 2023 •

edited

Loading

piotr-roslaniec Jul 13, 2023

derekpierre Jul 13, 2023

piotr-roslaniec commented Aug 1, 2023

Update Python typings #139

Update Python typings #139

Conversation

piotr-roslaniec commented Jul 10, 2023 • edited Loading

codecov-commenter commented Jul 10, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KPrasch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

derekpierre left a comment

Choose a reason for hiding this comment

piotr-roslaniec commented Jul 13, 2023

derekpierre Jul 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

piotr-roslaniec commented Aug 1, 2023

piotr-roslaniec commented Jul 10, 2023 •

edited

Loading

codecov-commenter commented Jul 10, 2023 •

edited

Loading

derekpierre Jul 13, 2023 •

edited

Loading